Skip to content

fix(speakers): restore activitiesCountAccurate conditional and totalActivities display#990

Merged
smarcet merged 1 commit into
masterfrom
fix/speakers-activities-count-display
Jun 18, 2026
Merged

fix(speakers): restore activitiesCountAccurate conditional and totalActivities display#990
smarcet merged 1 commit into
masterfrom
fix/speakers-activities-count-display

Conversation

@mulldug

@mulldug mulldug commented Jun 18, 2026

Copy link
Copy Markdown

ref: https://app.clickup.com/t/9014802374/86b9b1qrk

Summary

  • The merge of feature/speakers-submitters-activities-count (PR feat(speakers): display unique activities count on speakers/submitters list #933) into master dropped the second hunk of the summit-speakers-list-page.js conflict resolution: the activitiesCountAccurate flag computation and the conditional T.translate block that passes {activitiesQty} to the items_qty string.
  • This left master in a broken state: en.json defines items_qty expecting an {activitiesQty} interpolation argument, but the page code never computes or passes it — so the activities count was silently missing from the rendered string.
  • This fix restores the two missing hunks: destructuring excludedItems from getSubjectProps(), computing activitiesCountAccurate = selectedAll && excludedItems.length === 0, and switching to the conditional translate that passes activitiesQty when all items are selected with no exclusions.

Summary by CodeRabbit

  • Bug Fixes
    • Improved the accuracy of activity counts displayed on the speakers/submitters list page.
    • Updated the item quantity label to show selection count and activity count when data is accurate, with a fallback display when counts may be less reliable.

…ctivities display

The merge of feature/speakers-submitters-activities-count into master dropped
the second hunk of the summit-speakers-list-page conflict: the activitiesCountAccurate
flag and the conditional T.translate block that passes {activitiesQty} to the
items_qty string. This left master with an en.json expecting {activitiesQty} but
page code that never passed it, so the count was always silently missing.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

In the speakers/submitters list page render() method, totalActivities and excludedItems are now destructured from getSubjectProps(). A derived boolean activitiesCountAccurate is computed from selectedAll && excludedItems.length === 0. The items qty label conditionally renders different i18n keys depending on this flag and whether the source is speakers or submitters.

Changes

Conditional Activities Count Display

Layer / File(s) Summary
Derive activitiesCountAccurate and conditional qty label
src/pages/summit_speakers/summit-speakers-list-page.js
Destructures totalActivities and excludedItems from getSubjectProps(); derives activitiesCountAccurate = selectedAll && excludedItems.length === 0; replaces the unconditional items qty translation with a conditional that uses items_qty (passing both qty and activitiesQty) when accurate, and items_qty_no_activities (passing only qty) otherwise, with speaker/submitter-specific key variants.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • fntechgit/summit-admin#933: Directly related — both PRs modify the speakers/submitters list page to conditionally display the items qty label based on activity count accuracy derived from selectedAll and excludedItems.

Suggested reviewers

  • smarcet

Poem

🐇 Hop, hop — the count is now precise,
When all are selected and none excised,
The activities qty joins the fray,
Otherwise just qty saves the day.
A little flag makes numbers right —
The rabbit checks, and all looks bright! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: restoring the activitiesCountAccurate conditional and totalActivities display, which directly addresses the regression described in the PR objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/speakers-activities-count-display

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/pages/summit_speakers/summit-speakers-list-page.js (1)

718-718: ⚡ Quick win

Consider adding a defensive guard for excludedItems.

While this code restores previously working functionality, adding a defensive check would prevent potential runtime errors if excludedItems is unexpectedly undefined:

🛡️ Suggested defensive guard
-const activitiesCountAccurate = selectedAll && excludedItems.length === 0;
+const activitiesCountAccurate = selectedAll && excludedItems?.length === 0;

Or, if optional chaining isn't available:

-const activitiesCountAccurate = selectedAll && excludedItems.length === 0;
+const activitiesCountAccurate = selectedAll && excludedItems && excludedItems.length === 0;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/summit_speakers/summit-speakers-list-page.js` at line 718, The line
where activitiesCountAccurate is assigned accesses the length property of
excludedItems without checking if it exists first, which could cause a runtime
error if excludedItems is unexpectedly undefined. Add a defensive guard to
safely access the length property of excludedItems by using optional chaining
syntax (excludedItems?.length) or by explicitly checking that excludedItems
exists before accessing its length property in the condition that checks
selectedAll && excludedItems.length === 0.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/pages/summit_speakers/summit-speakers-list-page.js`:
- Line 718: The line where activitiesCountAccurate is assigned accesses the
length property of excludedItems without checking if it exists first, which
could cause a runtime error if excludedItems is unexpectedly undefined. Add a
defensive guard to safely access the length property of excludedItems by using
optional chaining syntax (excludedItems?.length) or by explicitly checking that
excludedItems exists before accessing its length property in the condition that
checks selectedAll && excludedItems.length === 0.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4e937b52-787b-47b6-9069-9d644924e944

📥 Commits

Reviewing files that changed from the base of the PR and between 8d5499c and b25a956.

📒 Files selected for processing (1)
  • src/pages/summit_speakers/summit-speakers-list-page.js

@smarcet smarcet left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@smarcet smarcet merged commit 23217a9 into master Jun 18, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants